Skip to content

chore(make): add CI-faithful verify targets to Makefile#19

Merged
inureyes merged 1 commit into
mainfrom
chore/makefile-verify-targets
May 18, 2026
Merged

chore(make): add CI-faithful verify targets to Makefile#19
inureyes merged 1 commit into
mainfrom
chore/makefile-verify-targets

Conversation

@inureyes
Copy link
Copy Markdown
Member

Summary

Adds a verify* family of Makefile targets that reproduce the GitHub Actions clippy + test (macOS ARM64) job exactly. The existing clippy / test / ci / pre-commit targets are weaker than CI in three ways that have repeatedly bitten us:

  1. Default features — without --features metal,accelerate, large gated regions of mlxcel-core (parts of cache/turbo/quant, the attention-dispatch helpers, the metal/accelerate-only branches in generate.rs) aren't even type-checked locally.
  2. -W warnings instead of -D warnings — local clippy silently passes on regressions that CI will reject.
  3. Debug-mode tests — CI runs cargo test --release, which exercises different optimisation paths.

This gap cost us three CI rounds (~45 min wasted on the self-hosted runner) while shipping the gpt-oss bf16 fix (#17) and the clippy 1.95 sweep (#18). The new targets close it.

New targets

Target Command
verify-fmt cargo fmt --all -- --check
verify-clippy cargo clippy --all-targets --features metal,accelerate -- -D warnings
verify-test cargo test --release --features metal,accelerate
verify runs the three above (recommended before push)
verify-clean cargo clean + verify (use when clippy's per-crate cache may be hiding a regression after editing shared mlxcel-core code)

Not changed

The existing looser targets (fmt-check, clippy, test, ci, pre-commit) are preserved — they remain useful for fast inner-loop iteration where strict CI parity isn't needed. The header comment block above the verify* block explains the distinction so the two sets don't accidentally get collapsed later.

Test plan

  • make verify-fmt succeeds on a clean tree
  • Manual review of each command vs .github/workflows/ci.yml lines 77, 102, 104
  • Will exercise make verify end-to-end in the next "before push" cycle

The existing `clippy`, `test`, `ci`, and `pre-commit` targets all run
weaker checks than `.github/workflows/ci.yml` does (default features,
`-W warnings` instead of `-D warnings`, debug-mode tests). That gap let
clippy and build regressions slip into PRs and only surface on the
self-hosted macOS runner ~15 minutes later — which we hit three times in
a row while shipping the gpt-oss bf16 fix and the clippy 1.95 sweep.

This adds a parallel set of `verify*` targets that mirror the CI job
step-for-step:

- `verify-fmt`    — cargo fmt --all -- --check
- `verify-clippy` — cargo clippy --all-targets --features metal,accelerate -- -D warnings
- `verify-test`   — cargo test --release --features metal,accelerate
- `verify`        — runs the three above; recommended before push
- `verify-clean`  — cargo clean + verify, for when clippy's per-crate
  cache may be hiding a regression in shared mlxcel-core code

The header comment block explains exactly why these differ from the
existing looser targets, so future readers don't accidentally collapse
the two sets back together. The looser `clippy` / `test` / `ci` /
`pre-commit` targets are left untouched — they remain useful for fast
inner-loop iteration where CI parity isn't needed.
@inureyes inureyes added status:review Under review type:chore Maintenance tasks (build, CI, etc.) priority:medium Medium priority labels May 18, 2026
@inureyes inureyes merged commit 9e55e2c into main May 18, 2026
5 checks passed
@inureyes inureyes deleted the chore/makefile-verify-targets branch May 18, 2026 11:33
@inureyes inureyes self-assigned this May 18, 2026
@inureyes inureyes added status:done Completed and removed status:review Under review labels May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:medium Medium priority status:done Completed type:chore Maintenance tasks (build, CI, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant